Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rename appcenter.podspec to appcenter-core to avoid conflicts #789

Merged
merged 8 commits into from
Jan 8, 2020
Merged

Rename appcenter.podspec to appcenter-core to avoid conflicts #789

merged 8 commits into from
Jan 8, 2020

Conversation

russelarms
Copy link
Contributor

@russelarms russelarms commented Dec 20, 2019 •
edited
Loading

Things to consider before you submit the PR:

  • Has CHANGELOG.md been updated?
  • Are tests passing locally?
  • Are the files formatted correctly?
  • Did you test your change with either the sample apps that are included in the repository or with a blank app that uses your change?

Description

Rename appcenter.podspec to appcenter-core to avoid conflicts.

Repro of the bug:
react-native init newapp && cd newapp
npm install appcenter appcenter-analytics appcenter-crashes --save-exact
cd ios && pod install && cd ..
rios
cd ios && rm -rf Pods && rm Podfile.lock && pod instal

Related PRs or issues

AB#71995
Github issue

@russelarms russelarms marked this pull request as ready for review December 20, 2019 16:56
@russelarms
Copy link
Contributor Author

I've tested it with the TestApp and with a new sample app. In both cases, it worked fine.

@russelarms
Copy link
Contributor Author

We have to test it with RN < 0.60, as for now, I have some issues with it. Maybe they are local.

Ruslan Urmeev and others added 2 commits December 23, 2019 13:30
@russelarms
Copy link
Contributor Author

I've tested it on a sample app built with RN 0.59.9, works fine. So it's ready to be merged now.

@dhei dhei changed the title Rename appcenter.podspec to appcenter-rn to avoid conflicts Rename appcenter.podspec to appcenter-core to avoid conflicts Dec 24, 2019
@Jarred-Sumner
Copy link

excited for this to merge

ilyashumihin
ilyashumihin previously approved these changes Dec 30, 2019
View reviewed changes
MatkovIvan
MatkovIvan previously approved these changes Dec 30, 2019
View reviewed changes
Jamminroot
Jamminroot previously approved these changes Dec 30, 2019
View reviewed changes
guperrot
guperrot reviewed Jan 6, 2020
View reviewed changes
CHANGELOG.md Show resolved Hide resolved
@jaeklim
Copy link
Contributor

jaeklim commented Jan 6, 2020

This PR contains two items. Let's avoid combining multiple items in a PR.

dhei
dhei reviewed Jan 6, 2020
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@annakocheshkova
Copy link

annakocheshkova commented Jan 7, 2020 •
edited
Loading

@jaelim these two fixes only work in a bundle. One is not enough without the other.

guperrot
guperrot suggested changes Jan 7, 2020
View reviewed changes
Copy link
Member

@guperrot guperrot left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise looks good to me.

@jaeklim
Copy link
Contributor

jaeklim commented Jan 7, 2020

@annakocheshkova Well, it seems to me like one name change and then one static framework change on top of name change. Am I wrong?

@guperrot
Copy link
Member

guperrot commented Jan 7, 2020

According to me it would be counter productive to split PRs at this point but let's keep that in mind for future PRs. All files have been already reviewed anyway.

@dhei
Update CHANGELOG.md
3593964 
Co-Authored-By: Di Hei <[email protected]>
@russelarms
Copy link
Contributor Author

russelarms commented Jan 8, 2020 •
edited
Loading

  • applied changelog fix
  • added a comment inside the podspec file
  • tested the change with both cocoapods v.1.7.5 and v1.8.4

@guperrot guperrot merged commit 048a5f6 into develop Jan 8, 2020
@guperrot guperrot deleted the fix/rename-appcenter-podspec branch January 8, 2020 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@annakocheshkova annakocheshkova annakocheshkova left review comments

@jaeklim jaeklim jaeklim left review comments

@dhei dhei dhei approved these changes

@MatkovIvan MatkovIvan MatkovIvan approved these changes

@guperrot guperrot guperrot approved these changes

@ilyashumihin ilyashumihin ilyashumihin left review comments

@Jamminroot Jamminroot Jamminroot left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@russelarms @Jarred-Sumner @jaeklim @annakocheshkova @guperrot @MatkovIvan @dhei @Jamminroot @ilyashumihin